Repository navigation
fix: release stream ids on request setup failure - #1008
dkropachev merged 1 commit into
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the main change: releasing stream IDs when request setup fails. Description check The description directly matches the changeset and states the failure cases, ownership behavior, regression tests, and validation command. Linked Issues check Issue #1002requires release of each concrete stream ID after acquisition when frame creation or callback registration fails.InFlightHandler.writereleases the ID infinallywhen no callback owns…Out of Scope Changes check The changes are limited to InFlightHandlercleanup logic and focusedInFlightHandlerTestregression coverage. The implementation and tests directly support issue#1002.Full details: Linked Issues check
-
Explanation
Issue
#1002requires release of each concrete stream ID after acquisition when frame creation or callback registration fails.InFlightHandler.writereleases the ID infinallywhen no callback owns it. It preserves the existing callback ownership on duplicate registration. The added tests cover frame failure, registration failure, ID reuse, and single release of the original request.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
c11ba9b to
7946c1e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## scylla-4.x #1008 +/- ##
================================================
+ Coverage 84.82% 84.86% +0.03%
- Complexity 8629 8635 +6
================================================
Files 848 848
Lines 31033 31038 +5
Branches 4281 4282 +1
================================================
+ Hits 26323 26339 +16
+ Misses 4710 4699 -11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can import rules and skills only from the repositories your team follows Powered by Qodo |
Leaving these methods unchanged. The repository Javadoc guidance makes documentation optional for internal packages and does not require every method to be documented. This PR changes internal code and tests; the ownership comments and regression-test names describe the behavior. |
An acquired stream ID has consumed its reservation before frame creation and callback registration can fail. Release the concrete ID until a callback owns it, while preserving IDs owned by an existing callback. Cover frame creation and registration failures, verify reuse of the released ID, and preserve the original request and response lifecycle. Fixes scylladb#1002
60607b0 to
c2df2e2
Compare
Summary
Fixes #1002.
Tests
JAVA_HOME=.../11.0.29-tem mvn -pl core -am -DskipITs -Dtest=InFlightHandlerTest -Dsurefire.failIfNoSpecifiedTests=false test